feat: (PRO-262) Implement usage limit feature with Redis support by dev-jodee · Pull Request #215 · solana-foundation/kora

dev-jodee · 2025-09-02T21:39:49Z

Removed redis-test dependency from Cargo files.
Updated Docker configuration to use a custom Redis configuration file.
Introduced UsageLimitConfig to manage per-wallet transaction limits.
Added UsageTracker for enforcing usage limits based on transactions.
Implemented UsageStore trait with Redis and in-memory implementations for tracking usage.
Enhanced RPC methods to check usage limits before processing transactions.
Added tests for usage limit functionality and configuration parsing.
Updated configuration validation to include usage limit settings.

Important

Implements per-wallet transaction usage limits with Redis support, updating configuration, RPC methods, and tests.

Behavior:
- Introduces UsageLimitConfig in config.rs for per-wallet transaction limits.
- Adds UsageTracker in usage_limit/usage_tracker.rs to enforce limits.
- Implements UsageStore trait with RedisUsageStore and InMemoryUsageStore in usage_limit/usage_store.rs.
- Enhances RPC methods in estimate_transaction_fee.rs, sign_and_send_transaction.rs, sign_transaction.rs, sign_transaction_if_paid.rs, and transfer_transaction.rs to check usage limits.
Configuration:
- Updates kora.toml and redis.conf for Redis configuration.
- Adds validation for usage limit settings in cache_validator.rs and config_validator.rs.
Testing:
- Adds tests for usage limit functionality in usage_tracker.rs and usage_store.rs.
- Updates existing tests to include usage limit checks.
Misc:
- Removes redis-test dependency from Cargo files.
- Updates Docker configuration to use custom Redis config file.

^{This description was created by}^{for 57402f7. You can customize this summary. It will automatically update as commits are pushed.}

📊 Test Coverage

Coverage: 85.8%

View Detailed Coverage Report

- Removed `redis-test` dependency from Cargo files. - Updated Docker configuration to use a custom Redis configuration file. - Introduced `UsageLimitConfig` to manage per-wallet transaction limits. - Added `UsageTracker` for enforcing usage limits based on transactions. - Implemented `UsageStore` trait with Redis and in-memory implementations for tracking usage. - Enhanced RPC methods to check usage limits before processing transactions. - Added tests for usage limit functionality and configuration parsing. - Updated configuration validation to include usage limit settings.

linear · 2025-09-02T21:39:51Z

PRO-262 feat: per-user rate-limiting

ellipsis-dev

Important

Looks good to me! 👍

Reviewed everything up to 09075f3 in 36 seconds. Click for details.

Reviewed 1071 lines of code in 20 files
Skipped 0 files when reviewing.
Skipped posting 1 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. docker-compose.yml:8

Draft comment:
Ensure that the custom Redis configuration file (./redis.conf) exists in the project root and is maintained for production–consider externalizing config options if needed.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 20% <= threshold 50% The comment is asking the author to ensure the existence and maintenance of a configuration file, which is a form of asking for confirmation or verification. This violates the rule against asking the author to ensure something is done. The suggestion to externalize config options is a valid suggestion, but the main part of the comment is not compliant with the rules.

Workflow ID: wflow_eusUrGAH5yWeFaLA

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

ellipsis-dev

Important

Looks good to me! 👍

Reviewed 7adea1e in 31 seconds. Click for details.

Reviewed 7 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 1 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. .github/badges/coverage.json:1

Draft comment:
Coverage badge updated to '85.7%'. Confirm this reflects the actual metrics and that coverage automation is in sync.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_frEKkWUsncXpBSZY

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

amilz

LMK if you want to sync up on any of these:

Rate Limit Expiration: my big thing here is that rate limits seem to be permenant--i think we need a TTL or something like we have on hmac in our config so we could set a rate limit per unit time (e.g., x requests/min). This would allow purging old keys too. Alternatively we need to make sure that clear gets implemented somewhere (or at least admin has endpoint to run clear otherwise I don't think it's used.
Monitoring: We should probably add monitoring to @crates/lib/src/metrics/mod.rs (e.g., users hitting their limits)
Non-wallet-based rate limiting: (e.g., IP)--IMO this would be a bit more effective at limiting requests/avoiding wallet rotation, etc. If you do this, then I think you could make rate limits global (rather than for each transaction methods). You'd be able to do validation in your middleware here (@crates/lib/src/rpc_server/server.rs).
Validation: - since we have this option now, IMO makes sense that we'd warn if a config is tested that does not have per-user rate limits
Docs/Samples: Once this PR is locked/merged--note to me to update docs/sample kora.tomls
Future/Backlog Ideas:
- maybe we have a method where user can check there rate limit
- classes of users/for different rate limit tiers

docker-compose.yml

crates/lib/src/config.rs

crates/lib/src/usage_limit/usage_tracker.rs

ellipsis-dev

Caution

Changes requested ❌

Reviewed ae04b9d in 2 minutes and 8 seconds. Click for details.

Reviewed 415 lines of code in 12 files
Skipped 0 files when reviewing.
Skipped posting 4 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. crates/lib/src/usage_limit/usage_tracker.rs:88

Draft comment:
The function 'extract_transaction_sender' returns Ok(None) when all signers are Kora fee payers. Ensure this bypass is the intended behavior; if user-initiated transactions should always be subject to limits, consider flagging this scenario explicitly.
Reason this comment was not posted:
Comment looked like it was already resolved.

2. kora.toml:96

Draft comment:
The usage limit configuration section is commented out. Confirm whether this is intentional for default deployments or if users should be prompted to uncomment and configure usage limiting.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is exactly the type of comment we should delete. It's asking for confirmation of intention ("Confirm whether this is intentional...") which violates our rules. If the author added it as commented code, we should assume they did so intentionally. Additionally, it's asking about documentation/configuration choices rather than pointing out a clear code issue. Maybe this is actually a security concern since usage limiting could be important for preventing abuse? Even if usage limiting is important, the author clearly made a deliberate choice to add this as commented example config. If it needs to be enabled by default, that should be a separate feature request, not a PR comment. Delete this comment. It violates our rule about not asking for confirmation of intentions, and doesn't point out a clear code issue that needs fixing.

3. redis.conf:35

Draft comment:
Review the Redis memory management settings. The maxmemory value of 256mb and the 'volatile-ttl' eviction policy should be verified against your production workload to ensure they are appropriate.
Reason this comment was not posted:
Confidence changes required: 30% <= threshold 50% None

4. crates/lib/src/admin/token_util.rs:30

Draft comment:
Typographical error: 'funciton' is misspelled; please change it to 'function'.
Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_gNiG8owR2eBXMkkx

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

crates/lib/src/usage_limit/usage_tracker.rs

- Config validation - Better fallback handling - Some flakey tests fixed

ellipsis-dev

Caution

Changes requested ❌

Reviewed 72db4a3 in 2 minutes and 59 seconds. Click for details.

Reviewed 549 lines of code in 6 files
Skipped 0 files when reviewing.
Skipped posting 4 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. crates/lib/src/rpc_server/method/transfer_transaction.rs:160

Draft comment:
Tests now use update_config with ConfigMockBuilder instead of setup_or_get_test_config. Ensure that update_config resets the configuration state consistently across tests.
Reason this comment was not posted:
Confidence changes required: 50% <= threshold 50% None

2. crates/lib/src/usage_limit/usage_store.rs:112

Draft comment:
ErrorUsageStore correctly simulates Redis errors. Consider simulating a more realistic stateful behavior (e.g., maintaining a counter) for non-error cases if deeper testing is needed.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50% None

3. crates/lib/src/usage_limit/usage_tracker.rs:49

Draft comment:
The fallback_if_unavailable mechanism in UsageTracker is useful to allow transactions when store errors occur. However, document and review potential security implications of bypassing usage limits when Redis errors occur.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 20% <= threshold 50% The comment suggests documenting and reviewing potential security implications, which is similar to asking the author to ensure certain behavior. This violates the rule against asking the author to confirm or ensure behavior. The comment does not provide a specific code suggestion or ask for a specific test to be written.

4. crates/lib/src/validator/cache_validator.rs:40

Draft comment:
CacheValidator tests the Redis connection, but consider adding a timeout or more detailed error context to handle prolonged connection attempts when Redis is unreachable.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The suggestion about timeouts is reasonable since hanging connections could be problematic. However, the deadpool_redis library likely has default timeouts. The error messages already provide basic context. The comment is somewhat speculative - it's suggesting improvements without clear evidence of an actual problem. I may be underestimating the importance of explicit timeouts in a validator. Connection timeouts could significantly impact startup time if Redis is down. While timeouts are important, this is a validator that runs at startup, not a critical runtime path. The default timeouts from deadpool_redis should be sufficient for this use case. The comment makes reasonable suggestions but they are more like optional improvements rather than clear issues that need to be fixed. The current implementation is acceptable.

Workflow ID: wflow_OOlhDJDcXbd2zjzE

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

crates/lib/src/validator/config_validator.rs

ellipsis-dev

Important

Looks good to me! 👍

Reviewed 57402f7 in 36 seconds. Click for details.

Reviewed 7 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 1 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. .github/badges/coverage.json:1

Draft comment:
Badge updated to reflect new coverage percentage. Ensure this value is in sync with your automated tests.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_p4OYNinp8eggrNbL

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

dev-jodee requested a review from amilz September 2, 2025 21:39

ellipsis-dev bot reviewed Sep 2, 2025

View reviewed changes

Update coverage badge [skip ci]

7adea1e

ellipsis-dev bot reviewed Sep 2, 2025

View reviewed changes

amilz reviewed Sep 3, 2025

View reviewed changes

docker-compose.yml Show resolved Hide resolved

crates/lib/src/config.rs Outdated Show resolved Hide resolved

crates/lib/src/usage_limit/usage_tracker.rs Outdated Show resolved Hide resolved

crates/lib/src/usage_limit/usage_tracker.rs Outdated Show resolved Hide resolved

PR comments + tested locally

ae04b9d

ellipsis-dev bot reviewed Sep 3, 2025

View reviewed changes

crates/lib/src/usage_limit/usage_tracker.rs Outdated Show resolved Hide resolved

PR fixes

72db4a3

- Config validation - Better fallback handling - Some flakey tests fixed

amilz self-requested a review September 3, 2025 14:00

amilz approved these changes Sep 3, 2025

View reviewed changes

ellipsis-dev bot reviewed Sep 3, 2025

View reviewed changes

crates/lib/src/validator/config_validator.rs Show resolved Hide resolved

Update coverage badge [skip ci]

57402f7

ellipsis-dev bot reviewed Sep 3, 2025

View reviewed changes

dev-jodee merged commit c2843ac into main Sep 3, 2025

dev-jodee deleted the feat/pro-262-user-rate-limits branch January 6, 2026 19:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: (PRO-262) Implement usage limit feature with Redis support#215

feat: (PRO-262) Implement usage limit feature with Redis support#215
dev-jodee merged 5 commits intomainfrom
feat/pro-262-user-rate-limits

dev-jodee commented Sep 2, 2025 •

edited by ellipsis-dev bot

Loading

Uh oh!

linear bot commented Sep 2, 2025

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

amilz left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dev-jodee commented Sep 2, 2025 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Test Coverage

Uh oh!

linear bot commented Sep 2, 2025

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

amilz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dev-jodee commented Sep 2, 2025 •

edited by ellipsis-dev bot

Loading